Skip to content

feat(config): add v6.1 ruleset and pin RNG seeds from submission ruleset#389

Merged
arekay-nv merged 8 commits into
mainfrom
feat/random-seeds
Jul 7, 2026
Merged

feat(config): add v6.1 ruleset and pin RNG seeds from submission ruleset#389
arekay-nv merged 8 commits into
mainfrom
feat/random-seeds

Conversation

@arekay-nv

Copy link
Copy Markdown
Collaborator

Adds the MLPerf Inference v6.1 RoundRuleset (schedule/sample_index seeds from loadgen/mlperf.conf; per-model latency targets unchanged from v5.1) and makes it CURRENT. The registry now registers every known round by version, so both v5.1 and v6.1 are resolvable.

When a config carries submission_ref, _resolve_and_validate now pins the runtime RNG seeds from the selected ruleset during construction — before the config is dumped to the report dir, so the persisted config.yaml, RuntimeSettings, and report all reflect the seeds that actually ran. The ruleset value wins over any user-supplied seed (mirrors LoadGen locking core seeds from user.conf). Lenient by design: an unregistered ruleset leaves the config unchanged, so placeholder/non-submission configs are unaffected.

Duration/latency overrides and the runtime-path integration (surfacing ruleset latency targets in the report) are deferred to a follow-up.

What does this PR do?

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor/cleanup

Related issues

Testing

  • Tests added/updated
  • All tests pass locally
  • Manual testing completed

Checklist

  • Code follows project style
  • Pre-commit hooks pass
  • Documentation updated (if needed)

Adds the MLPerf Inference v6.1 RoundRuleset (schedule/sample_index seeds from
loadgen/mlperf.conf; per-model latency targets unchanged from v5.1) and makes
it CURRENT. The registry now registers every known round by version, so both
v5.1 and v6.1 are resolvable.

When a config carries submission_ref, _resolve_and_validate now pins the
runtime RNG seeds from the selected ruleset during construction — before the
config is dumped to the report dir, so the persisted config.yaml,
RuntimeSettings, and report all reflect the seeds that actually ran. The
ruleset value wins over any user-supplied seed (mirrors LoadGen locking core
seeds from user.conf). Lenient by design: an unregistered ruleset leaves the
config unchanged, so placeholder/non-submission configs are unaffected.

Duration/latency overrides and the runtime-path integration (surfacing ruleset
latency targets in the report) are deferred to a follow-up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arekay-nv arekay-nv requested a review from a team July 2, 2026 19:59
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@github-actions github-actions Bot requested a review from nvzhihanj July 2, 2026 19:59

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the MLPerf Inference v6.1 ruleset, including its official RNG seeds, and updates the default submission template to reference v6.1. It also implements a mechanism in BenchmarkConfig to automatically override and pin runtime RNG seeds based on the selected submission ruleset, ensuring compliance. Feedback is provided regarding the definition of _v6_1, where copying the mutable benchmark_rulesets dictionary instead of sharing its reference with _v5_1 is recommended to prevent potential side effects.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/inference_endpoint/config/rulesets/mlcommons/rules.py Outdated

@arekay-nv arekay-nv left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Council — Multi-AI Code Review

Reviewed by: Codex + Claude | Depth: thorough

Codex: no actionable correctness issues. Claude: 5 posted (1 shared-dict finding on rules.py:282 was dropped as a duplicate of the existing gemini-code-assist comment). See the summary comment below for the tiered breakdown.

Comment thread src/inference_endpoint/config/schema.py Outdated
Comment thread tests/unit/config/rulesets/mlcommons/test_rules.py
Comment thread src/inference_endpoint/config/ruleset_registry.py Outdated
Comment thread src/inference_endpoint/config/rulesets/mlcommons/rules.py Outdated
Comment thread src/inference_endpoint/config/schema.py Outdated
@arekay-nv

Copy link
Copy Markdown
Collaborator Author

Review Council — Multi-AI Code Review

Reviewed by: Codex (gpt-5.5, xhigh) + Claude | Depth: thorough

Codex: "changes consistently register the new MLCommons round and apply the selected ruleset seeds into runtime configuration without introducing an evident functional regression. No actionable correctness issues."

Claude: 6 findings; 1 dropped as a duplicate of the existing gemini-code-assist comment on rules.py:282 (shared mutable-dict reference). 5 posted inline.

No critical/high issues. The core logic (frozen-model mutation via model_copy + object.__setattr__, lenient resolution, ruleset-wins seed lock, before-dump ordering) reviewed clean under both reviewers.

🟡 Should Fix (medium)

# File Line Category Reviewer Summary
1 config/schema.py 1055 testing Claude is not None seed branches + if not updates early-exit are untested — a None-seed ruleset is a documented, supported state

🔵 Consider (low)

# File Line Category Reviewer Summary
2 .../mlcommons/test_rules.py 147 testing Claude benchmark_rulesets == assertion is identity-trivially true (same dict object) — verifies nothing
3 config/ruleset_registry.py 85 design Claude No duplicate-version guard — a future round sharing a version string would be silently overwritten
4 .../mlcommons/rules.py 280 docs/verify Claude Compliance-locked seeds match docs/RANDOM_SEEDS.md (@ v6.0.0pre) but should be confirmed against the tagged v6.1 mlperf.conf
5 config/schema.py 1068 error-handling Claude logger.info("Pinned RNG seeds …") fires multiple times per run (with_updates re-validates) — consider debug

Related: the existing gemini-code-assist comment on rules.py:282 (copy the shared benchmark_rulesets dict) would also resolve finding #2 if applied.

Commit hygiene: 1 commit, 0 fixups — clean.

Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>

@arekay-nv arekay-nv left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Council — Multi-AI Code Review (re-review of 346bd6b)

Reviewed by: Codex (gpt-5.5, xhigh) + Claude | Depth: thorough

Codex: no introduced correctness issues. Claude: 7 findings — 6 posted inline below, 1 summary-only (outside diff). The prior run's 6 findings are all resolved by 346bd6b. See the summary comment for the tiered breakdown.

Comment thread src/inference_endpoint/config/rulesets/mlcommons/rules.py Outdated
Comment thread src/inference_endpoint/config/schema.py Outdated
Comment thread src/inference_endpoint/config/rulesets/mlcommons/rules.py Outdated
Comment thread src/inference_endpoint/config/ruleset_registry.py
Comment thread src/inference_endpoint/config/schema.py Outdated
Comment thread tests/unit/config/test_schema.py Outdated
@arekay-nv

Copy link
Copy Markdown
Collaborator Author

Review Council — Multi-AI Code Review (re-review of 346bd6b)

Reviewed by: Codex (gpt-5.5, xhigh) + Claude | Depth: thorough

Codex: "I did not find any introduced correctness issues in the diff. The new ruleset registration and seed pinning logic appears consistent with the updated tests and existing config/runtime flow."

Claude: 7 findings — 6 posted inline, 1 summary-only (outside the diff). No critical/high. The core logic (frozen-model mutation via model_copy + object.__setattr__, ruleset-wins seed lock, before-dump ordering) reviewed clean under both reviewers.

Prior round resolved: all 6 findings from the first Review Council run are addressed by 346bd6b — shared-dict → copied, duplicate-version guard added, logger.infologger.debug, and the None-seed + concrete-target tests added.

🟡 Should Fix (medium)

# File Line Category Summary
1 config/rulesets/mlcommons/rules.py 276 documentation Comment references docs/RANDOM_SEEDS.md, which is not tracked in the repo — a dangling ref that already misled a reviewer (violates AGENTS.md). Commit the doc or inline the rationale.
2 config/schema.py 1046 data-integrity A type=SUBMISSION config with an unregistered submission_ref.ruleset silently falls back to default seeds (42/42) → non-compliant submission that looks valid. Make it a hard error for SUBMISSION.

🔵 Consider (low)

# File Line Category Summary
3 config/rulesets/mlcommons/rules.py 283 design rules.copy() is a 2-level copy, not the "Deep copy" the commit claims — leaf rulesets (with a mutable reported_metrics list) are still shared with v5.1. Latent; deepcopy or a shared-immutable note.
4 config/ruleset_registry.py 52 api-contract New duplicate guard makes the module non-idempotent — a re-import / importlib.reload now raises mid-import instead of silently overwriting.
5 config/schema.py 1062 error-handling model_copy(update=...) bypasses validation and field-name checks; the hardcoded "..._random_seed" keys are decoupled from RuntimeConfig — a future field rename fails silently.
6 tests/unit/config/test_schema.py 827 testing The mirror partial-None case (scheduler set, sample_index_rng_seed=None) is untested; that is not None False branch is only hit with an already-empty updates.
7 tests/unit/config/test_schema.py 167 documentation Pre-existing test pins ruleset="mlperf-inference-v6.0", which is not registered (only v5.1/v6.1). Outside the diff, so not posted inline — with the new override it now logs a "not registered" warning every run. Bump to v6.1.

Commit hygiene: 2 commits, 0 fixups — clean.

Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
@arekay-nv arekay-nv requested a review from viraatc July 7, 2026 01:18
@arekay-nv

Copy link
Copy Markdown
Collaborator Author

with the following section in the client config file (for deepseek for instance):

submission_ref:
  model: deepseek-r1 
  ruleset: mlperf-inference-v6.1

results_summary.json should contain the following lines:

"seeds": {
    "scheduler_random_seed": 16159082839903944936,
    "dataloader_random_seed": 2747215439041700203,
    "warmup_random_seed": 42
  }

@arekay-nv arekay-nv left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Council — Multi-AI Code Review (independent)

Reviewed by Codex + Claude | depth: thorough | posted inline: 4

Codex reported no correctness issues. Claude's findings below (none critical/high — the PR is fundamentally sound).

Comment thread src/inference_endpoint/config/schema.py
Comment thread src/inference_endpoint/config/ruleset_registry.py
Comment thread tests/unit/config/test_schema.py
Comment thread src/inference_endpoint/config/rulesets/mlcommons/rules.py
@arekay-nv

Copy link
Copy Markdown
Collaborator Author

Review Council — Multi-AI Code Review (independent)

Reviewed by Codex + Claude | depth: thorough

These reviews were run independently of any prior context — Codex in an isolated worktree, Claude fresh — assessing the diff on its own merits (no assumed verification).

Codex: no correctness issues found (reviewed against merge-base(main)).
Claude: 7 findings — none critical/high; the PR is fundamentally sound. 4 posted inline, 2 already covered by existing comments, 1 off-hunk (below).

🟡 Should Fix (medium)

# File Line Category Status Summary
1 config/schema.py 1048 api-contract ✅ inline Old type=submission configs with ruleset: mlperf-inference-v6.0 (the value the previous template shipped) now hard-fail to load — worth a migration/changelog note.
2 config/templates/submission_template.yaml 51 documentation ⚠️ off-hunk The scheduler_random_seed: 42 / dataloader_random_seed: 42 comments are misleading: this template is type: submission + v6.1, so the seeds are pinned from the ruleset and 42 is never used. Drop or annotate the fields.
3 config/schema.py 1063 error-handling 🔁 already-commented User seeds are silently overridden and logged only at debug; the override fires for any config with a submission_ref (only the error path is gated to SUBMISSION), so offline/online configs referencing a ruleset also get seeds rewritten.
4 config/ruleset_registry.py 96 design ✅ inline setdefault bypasses the new duplicate guard — a duplicate .version in ALL_ROUNDS is silently dropped; correctness depends on test_round_versions_are_unique running.

🔵 Consider (low)

# File Line Category Status Summary
5 config/schema.py 1075 design 🔁 already-commented The unknown-runtime-fields raise is unreachable (updates only ever holds two valid hard-coded keys) and wouldn't catch the rename it claims to guard.
6 tests/unit/config/test_schema.py 803 testing ✅ inline Untested new branches: registry idempotency (double-call), and the SUBMISSION unregistered-ruleset raise via the from-config YAML path.
7 config/rulesets/mlcommons/rules.py 287 design ✅ inline deepcopy of benchmark_rulesets keeps working only because OptimizationPriority is an enum (deepcopy(member) is member); prefer a key-preserving copy.

Commit hygiene: 3 commits, clean — no squash needed.

arekay-nv added 3 commits July 7, 2026 11:05
Signed-off-by: arekay-nv <230885705+arekay-nv@users.noreply.github.com>
Signed-off-by: arekay-nv <230885705+arekay-nv@users.noreply.github.com>
arekay-nv added 2 commits July 7, 2026 15:39
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
@arekay-nv arekay-nv merged commit f9e1aca into main Jul 7, 2026
9 checks passed
@arekay-nv arekay-nv deleted the feat/random-seeds branch July 7, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants